-
Notifications
You must be signed in to change notification settings - Fork 140
feat(auth):login and role-based authentication system #135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Warning Rate limit exceeded@Saahi30 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 53 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds Supabase-backed auth: example env and service key, a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LoginPage as Login Page
participant SupabaseAuth as Supabase Auth
participant ProfilesDB as Profiles DB
participant Router
User->>LoginPage: submit email & password
LoginPage->>SupabaseAuth: signInWithPassword()
alt auth success
SupabaseAuth-->>LoginPage: user object
LoginPage->>ProfilesDB: select role by user.id
alt profile found
ProfilesDB-->>LoginPage: profile (role)
LoginPage->>Router: navigate to role-specific home
else profile missing
LoginPage-->>User: show profile missing error
end
else auth failed
SupabaseAuth-->>LoginPage: error
LoginPage-->>User: show friendly auth error
end
sequenceDiagram
participant Browser
participant AuthGuard
participant Supabase as getCurrentUser()
participant Profiles as getUserProfile()
participant Router
Browser->>AuthGuard: access protected page
AuthGuard->>Supabase: getCurrentUser()
alt no user
AuthGuard->>Router: redirect /login
else user present
Supabase-->>AuthGuard: user
AuthGuard->>Profiles: getUserProfile()
alt role matches requiredRole
Profiles-->>AuthGuard: profile
AuthGuard-->>Browser: render children
else mismatch or missing
AuthGuard->>Router: redirect to actual-role home
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
frontend/app/globals.css (1)
6-30: Verify color palette completeness against PR design objectives.The primary color scale is correctly structured for Tailwind v4, but the PR objectives mention "purple-to-blue branding" whereas the CSS only defines a purple palette. There is no secondary blue/accent color scale defined.
Confirm whether:
- Blue colors are intentionally deferred to a later phase, or
- A complementary blue/accent palette should be added now (e.g.,
--secondary-*or--accent-*tokens).Additionally, verify that these color tokens are being actively consumed in the new authentication components (login, signup, and role-specific dashboards). If unused at this stage, they can be deferred.
frontend/middleware.ts (1)
8-19: Tidy up unused middleware flags
publicRoutes/isPublicRouteandisProtectedRouteare computed but never used, and the middleware always falls through toNextResponse.next(). Either wire these checks into a redirect/early return or drop the dead code so the intent of the middleware stays clear.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
backend/.env.example(1 hunks)backend/SQL(1 hunks)frontend/app/brand/home/page.tsx(1 hunks)frontend/app/creator/home/page.tsx(1 hunks)frontend/app/globals.css(1 hunks)frontend/app/login/page.tsx(1 hunks)frontend/app/page.tsx(1 hunks)frontend/app/signup/page.tsx(1 hunks)frontend/components/auth/AuthGuard.tsx(1 hunks)frontend/lib/auth-helpers.ts(1 hunks)frontend/middleware.ts(1 hunks)frontend/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-07T21:28:06.358Z
Learnt from: muntaxir4
Repo: AOSSIE-Org/InPactAI PR: 56
File: Backend/app/services/redis_client.py:1-4
Timestamp: 2025-05-07T21:28:06.358Z
Learning: Hardcoded Redis connection parameters in Backend/app/services/redis_client.py are intentional during development, with plans to implement environment variable configuration later during production preparation.
Applied to files:
backend/.env.example
🪛 dotenv-linter (4.0.0)
backend/.env.example
[warning] 5-5: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
[warning] 8-8: [UnorderedKey] The SUPABASE_KEY key should go before the SUPABASE_URL key
(UnorderedKey)
[warning] 10-10: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
[warning] 17-17: [UnorderedKey] The AI_API_KEY key should go before the GROQ_API_KEY key
(UnorderedKey)
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Nitpick comments (2)
backend/app/api/routes/auth.py (2)
6-8: Consider dependency injection for the Supabase client.While module-level initialization works, FastAPI best practices favor dependency injection for better testability and lifecycle management.
10-14: Consider stronger password requirements.A minimum password length of 8 characters meets basic standards but is below modern security best practices. Consider increasing to 12 or adding complexity requirements.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/app/api/routes/auth.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.3)
backend/app/api/routes/auth.py
36-36: Abstract raise to an inner function
(TRY301)
49-49: Abstract raise to an inner function
(TRY301)
52-52: Do not catch blind exception: Exception
(BLE001)
53-53: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
53-53: Use explicit conversion flag
Replace with conversion flag
(RUF010)
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
backend/.env.example(1 hunks)backend/app/api/routes/auth.py(1 hunks)backend/app/core/config.py(1 hunks)backend/app/main.py(2 hunks)backend/env_example(1 hunks)frontend/app/signup/page.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-05-07T21:28:06.358Z
Learnt from: muntaxir4
Repo: AOSSIE-Org/InPactAI PR: 56
File: Backend/app/services/redis_client.py:1-4
Timestamp: 2025-05-07T21:28:06.358Z
Learning: Hardcoded Redis connection parameters in Backend/app/services/redis_client.py are intentional during development, with plans to implement environment variable configuration later during production preparation.
Applied to files:
backend/.env.example
🪛 dotenv-linter (4.0.0)
backend/.env.example
[warning] 5-5: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
[warning] 8-8: [UnorderedKey] The SUPABASE_KEY key should go before the SUPABASE_URL key
(UnorderedKey)
[warning] 9-9: [UnorderedKey] The SUPABASE_SERVICE_KEY key should go before the SUPABASE_URL key
(UnorderedKey)
[warning] 11-11: [ExtraBlankLine] Extra blank line detected
(ExtraBlankLine)
[warning] 18-18: [UnorderedKey] The AI_API_KEY key should go before the GROQ_API_KEY key
(UnorderedKey)
🪛 Ruff (0.14.3)
backend/app/api/routes/auth.py
45-45: Abstract raise to an inner function
(TRY301)
61-61: Do not catch blind exception: Exception
(BLE001)
68-71: Abstract raise to an inner function
(TRY301)
72-72: Abstract raise to an inner function
(TRY301)
78-78: Use explicit conversion flag
Replace with conversion flag
(RUF010)
116-116: Abstract raise to an inner function
(TRY301)
117-117: Abstract raise to an inner function
(TRY301)
119-119: Abstract raise to an inner function
(TRY301)
125-125: Abstract raise to an inner function
(TRY301)
135-135: Use raise without specifying exception name
Remove exception name
(TRY201)
136-136: Do not catch blind exception: Exception
(BLE001)
137-137: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
137-137: Use explicit conversion flag
Replace with conversion flag
(RUF010)
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
backend/app/api/routes/auth.py (2)
2-2: Remove unused import.The
constrimport is not used anywhere in the file. All string constraints are defined usingFieldwith validation parameters.Apply this diff:
-from pydantic import BaseModel, EmailStr, constr,Field +from pydantic import BaseModel, EmailStr, Field
31-79: Signup logic is robust and well-structured.The endpoint correctly implements:
- Email verification flow (Supabase sends verification email automatically)
- Proper client separation (public for auth, admin for profile and rollback)
- Defensive error handling with retries for rollback operations
- Appropriate exception chaining to preserve error context
All previous critical issues have been addressed effectively.
Minor suggestion: Consider adding a small delay between rollback retry attempts to handle transient network issues more gracefully:
import asyncio for attempt in range(2): try: supabase_admin.auth.admin.delete_user(user.id) break except Exception as rollback_err: rollback_error = rollback_err if attempt == 0: await asyncio.sleep(0.5) # Brief delay before retry continue
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/app/api/routes/auth.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.3)
backend/app/api/routes/auth.py
46-46: Abstract raise to an inner function
(TRY301)
62-62: Do not catch blind exception: Exception
(BLE001)
69-72: Abstract raise to an inner function
(TRY301)
73-73: Abstract raise to an inner function
(TRY301)
79-79: Use explicit conversion flag
Replace with conversion flag
(RUF010)
117-117: Abstract raise to an inner function
(TRY301)
118-118: Abstract raise to an inner function
(TRY301)
120-120: Abstract raise to an inner function
(TRY301)
126-126: Abstract raise to an inner function
(TRY301)
136-136: Use raise without specifying exception name
Remove exception name
(TRY201)
137-137: Do not catch blind exception: Exception
(BLE001)
138-138: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
138-138: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🔇 Additional comments (3)
backend/app/api/routes/auth.py (3)
7-10: Excellent separation of concerns!Using separate clients for public auth flows and admin operations is the correct approach. This ensures that privileged operations like
delete_userand RLS-bypassed table access work reliably.
13-28: Well-structured request/response models.The Pydantic models include appropriate validation constraints (email format, minimum lengths, role pattern matching) and clear documentation. The field constraints align well with security best practices.
122-126: Profile fetching is correctly implemented.Using the admin client to fetch profile data ensures the query bypasses RLS. The defensive error handling and specific field selection are appropriate.
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/app/api/routes/auth.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.3)
backend/app/api/routes/auth.py
46-46: Abstract raise to an inner function
(TRY301)
62-62: Do not catch blind exception: Exception
(BLE001)
69-72: Abstract raise to an inner function
(TRY301)
73-73: Abstract raise to an inner function
(TRY301)
79-79: Use explicit conversion flag
Replace with conversion flag
(RUF010)
114-114: Do not catch blind exception: Exception
(BLE001)
118-118: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
119-119: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
121-121: Abstract raise to an inner function
(TRY301)
127-127: Abstract raise to an inner function
(TRY301)
139-139: Use explicit conversion flag
Replace with conversion flag
(RUF010)
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
backend/app/api/routes/auth.py (1)
113-124: Fix inconsistent exception handling pattern.The login endpoint catches broad
Exceptionand useshasattrchecks (line 122), while the signup endpoint properly catchesAuthApiErrordirectly (line 45). This inconsistency creates fragile error handling and violates the established pattern.Additionally:
AuthApiErroris already imported (line 3) but not used here- The comment on line 121 says "Import AuthApiError if available" which is confusing since it's already imported
- Missing exception chaining at lines 123-124 (Ruff B904 hints)
Apply this diff to align with the signup pattern:
# 1. Authenticate user try: auth_resp = supabase_public.auth.sign_in_with_password({ "email": payload.email, "password": payload.password }) user = getattr(auth_resp, "user", None) - except Exception as e: - # Supabase Python SDK v2 raises exceptions for auth errors - # Import AuthApiError if available - if hasattr(e, "code") and e.code == "email_not_confirmed": - raise HTTPException(status_code=403, detail="Please verify your email before logging in.") - raise HTTPException(status_code=401, detail=str(e)) + except AuthApiError as e: + if getattr(e, "code", None) == "email_not_confirmed": + raise HTTPException(status_code=403, detail="Please verify your email before logging in.") from e + raise HTTPException(status_code=401, detail=str(e)) from e
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/app/api/routes/auth.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.14.3)
backend/app/api/routes/auth.py
51-51: Abstract raise to an inner function
(TRY301)
67-67: Do not catch blind exception: Exception
(BLE001)
74-77: Abstract raise to an inner function
(TRY301)
78-78: Abstract raise to an inner function
(TRY301)
84-84: Use explicit conversion flag
Replace with conversion flag
(RUF010)
119-119: Do not catch blind exception: Exception
(BLE001)
123-123: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
124-124: Within an except clause, raise exceptions with raise ... from err or raise ... from None to distinguish them from errors in exception handling
(B904)
126-126: Abstract raise to an inner function
(TRY301)
132-132: Abstract raise to an inner function
(TRY301)
144-144: Use explicit conversion flag
Replace with conversion flag
(RUF010)
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
📝 Description
This PR implements a complete authentication system for InPactAI with role-based access control. The system supports two user types (Creator and Brand) with separate dashboards and includes signup, login, logout, and protected route functionality using Next.js 15, TypeScript, Supabase Auth, and Tailwind CSS v4.
Users can now:
🔧 Changes Made
Frontend (Next.js 15 + TypeScript)
New Pages:
/signup) - Complete registration flow with form validation/login) - Modern authentication interface/creator/home) - Protected dashboard for creators/brand/home) - Protected dashboard for brandsComponents & Helpers:
AuthGuardcomponent for route protection and role verificationauth-helpers.tswith utility functions:getCurrentUser()- Get authenticated usergetUserProfile()- Fetch user profile from databasesignOut()- Logout functionalitycheckUserRole()- Verify user rolesgetAuthErrorMessage()- User-friendly error mappingFeatures Implemented:
/creator/home, Brand →/brand/home)Design System:
globals.csswith custom primary color paletteDependencies Added:
zod- Schema validationreact-hook-form- Form management@hookform/resolvers- Zod integration📷 Screenshots or Visual Changes (if applicable)
Signup Page
Login Page
User Home
🔐 Authentication Flow
Signup: User → Form Validation → Supabase Auth → Create Profile → Success → Redirect to Login Login: User → Credentials → Verify → Fetch Role → Route to /creator/home or /brand/home Access: User → Protected Route → AuthGuard → Check Auth & Role → Allow or Redirect
✅ Checklist
Summary by CodeRabbit
New Features
Chores